Issue #55 #56
Issue #55 #56
Conversation
Ignore hidden excel sheets when importing Additional, trim cell values before processing
|
Hi @Developer-Matt , Thank you for your PR!, could you please provide one test for that case as well? |
|
Hi, |
|
All done. Please let me know if you would like any more additional specific test. Matt |
|
Thanks @Developer-Matt , I will look at the changes |
|
Hi @Developer-Matt , I found that you get the first visible sheet from the excel file. However, what if we have more than one visible sheets in an excel and we need to retrieve the second visible sheet among them? |
|
Hi, Ah yes I see your point, I have re written the method that finds which sheet to use, to now consider if a specified sheet index was requested. Have also created additional test cases for this. |
|
Thanks @Developer-Matt , I will merge your PR soon with small changes |
|
Hi @Developer-Matt , After testing the changes, I found that we should consider another way of implementing |
| @@ -39,6 +42,12 @@ | |||
|
|
|||
| <T> void unmarshal0(Class<T> type, Consumer<? super T> consumer, OPCPackage open) throws IOException, SAXException, OpenXML4JException { | |||
|
|
|||
| //ISSUE #55 | |||
| XSSFWorkbook wb = new XSSFWorkbook(open); | |||
There was a problem hiding this comment.
this is the statement where I get "gc exception". @Developer-Matt
There was a problem hiding this comment.
It kinda makes sense as to why it does that, but that's annoying. At the moment I am using my version and we are not importing that number of rows, so I would like to try and find a way to fix this before forced to limit only to xls. So unless there are other developers that need this functionally now, can you hold of on doing the pull and I will do some investigating and let you know what I find.
There was a problem hiding this comment.
no problem, I will inform you as well if I find something useful for this case.
There was a problem hiding this comment.
Hey, I have done some work on a different way to find hidden and visible sheets to avoid the GC exception. Can you please have a look at it when you get a chance and see if its an ok way to do it, and if it makes sense and if passes your 187k test. Have also included some big excel files for tests, hope they are not to large for repo.
The code still needs to be tidied up and made neater but just would like you to have a look first. If you happy with it I will go and tidy it all up and then can merge.
Thanks
There was a problem hiding this comment.
Hi @Developer-Matt , good news is that all tests pass successfully yay! :) , I will look at the changes you have made and will make some changes If necessary.
There was a problem hiding this comment.
Awesome. Cool, I will go clean up and comment my code quick
…it exceeded exception. Just proof of concept, still needs to be tidied up
|
Thank you very much @Developer-Matt , now merged! 🎉 |
|
Awesome thanks, I did just do another push, I tidied up and commented my code. Not sure if you would like the cleaner code, can create a new pull request but seems a waste, let me know. |
|
Hi @Developer-Matt , |
Ignore hidden excel sheets when importing
Additional, trim cell values before processing